-
Notifications
You must be signed in to change notification settings - Fork 332
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Mercenary #1448
base: master
Are you sure you want to change the base?
Mercenary #1448
Conversation
can you run the code through eslint |
Ran esLint - it didn't detect any issues other than not being able to find certain symbols (like Config/Town/Misc/me/print()/etc). |
well there seems to be lots of white space / new line issues in mercenary.js are you using the eslint settings for kolbot provided in the readme or some random one? |
Sorry, you're right, I missed the eslint configs from the readme. I re-ran and fixed them. |
Checked in a fix but somehow other stuff got rolled up into the push. I'll have to fix it. |
Removed commits that didn't belong in this PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see comments
throw new Error("Mercenary.hire: args.mercType is required"); | ||
} | ||
|
||
if (args.mercType < 1 || args.mercType === 4 || args.mercType > 5) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've seen this check done before, maybe change mercType to a get/set that auto validates the input so you don't need to sanitize it every time
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure that I understand what you mean by this or how you propose that it would be used.
|
||
switch (mercType) { | ||
case 1: | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
extra new line
} | ||
|
||
break; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
don't need newlines after breaks
if (args.variant === v) { | ||
break; | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove new line
} | ||
|
||
me.cancel(); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove new line
print("No mercs available"); | ||
|
||
me.cancel(); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove new line
revive: function () { | ||
if (!this.needMerc()) { | ||
print("I don't need to revive my merc"); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove new line
while (getTickCount() - tick < 2000) { | ||
if (this.getMerc()) { | ||
delay(Math.max(750, me.ping * 2)); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove new line
print("Unloading merc equipment"); | ||
var cursorItem; | ||
|
||
// ok this is a bit stupid |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you turn this into a loop that goes 4, 3, 1 since the code just repeats?
|
||
delay(1000 + me.ping * 2); | ||
|
||
removeEventListener("gamepacket", this._gamePacket); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
where is _gamePacket?
|
||
mercPacket: function (bytes) { | ||
if (bytes[0] !== 0x4e) { | ||
return; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is there any way to get available mercs without using packets?
possibly by parsing the list?
Highlights:
To specify which merc to use, modify config as follows:
Config.UseMerc={mercType: 1, variant: 11};
mercType is set to the act# - for example, 1 for act 1 rogue merc, 5 for act 5 barb
variant is set to the skill (see sdk/skills.txt) that you want your merc to have - in this case 11 for cold arrow.
Variant can be omitted if you don't care which skill the merc has.
Notes: